[X86][FastISel] Restore support for struct returns#194586
Merged
Conversation
After llvm#180322, X86 FastISel forces SDAG fallback for any call with a struct return. This caused major compile-time regressions for debug builds in Rust, where struct returns are very common. The type legality check should work on the de-aggregated types, not on the return type directly.
Member
|
@llvm/pr-subscribers-backend-x86 Author: Nikita Popov (nikic) ChangesAfter #180322, X86 FastISel forces SDAG fallback for any call with a struct return. This caused major compile-time regressions for debug builds in Rust, where struct returns are very common. The type legality check should work on the de-aggregated types, not on the return type directly. Full diff: https://github.com/llvm/llvm-project/pull/194586.diff 2 Files Affected:
diff --git a/llvm/lib/Target/X86/X86FastISel.cpp b/llvm/lib/Target/X86/X86FastISel.cpp
index d36a9581a3638..f91ef4abbdf27 100644
--- a/llvm/lib/Target/X86/X86FastISel.cpp
+++ b/llvm/lib/Target/X86/X86FastISel.cpp
@@ -21,6 +21,7 @@
#include "X86Subtarget.h"
#include "X86TargetMachine.h"
#include "llvm/Analysis/BranchProbabilityInfo.h"
+#include "llvm/CodeGen/Analysis.h"
#include "llvm/CodeGen/FastISel.h"
#include "llvm/CodeGen/FunctionLoweringInfo.h"
#include "llvm/CodeGen/MachineConstantPool.h"
@@ -3208,18 +3209,22 @@ bool X86FastISel::fastLowerCall(CallLoweringInfo &CLI) {
// the value from FuncInfo.ValueMap.
// However, i1 is promoted to i8 and return i8 defined by ABI, so FastISel can
// lower it without switching to DAGISel.
- MVT RetVT = MVT::Other;
- if (!isTypeLegal(CLI.RetTy, RetVT) && !CLI.RetTy->isVoidTy()) {
- if (RetVT == MVT::Other)
- return false; // Unknown type, let DAG ISel handle it.
-
- // RetVT is not MVT::Other, it must be simple now. It is something rely on
- // the logic of isTypeLegal().
- MVT ABIVT = TLI.getRegisterTypeForCallingConv(CLI.RetTy->getContext(),
- CLI.CallConv, RetVT);
- MVT RegVT = TLI.getRegisterType(CLI.RetTy->getContext(), RetVT);
- if (ABIVT != RegVT)
- return false;
+ SmallVector<Type *> RetTys;
+ ComputeValueTypes(DL, CLI.RetTy, RetTys);
+ for (Type *RetTy : RetTys) {
+ MVT RetVT = MVT::Other;
+ if (!isTypeLegal(RetTy, RetVT)) {
+ if (RetVT == MVT::Other)
+ return false; // Unknown type, let DAG ISel handle it.
+
+ // RetVT is not MVT::Other, it must be simple now. It is something rely on
+ // the logic of isTypeLegal().
+ MVT ABIVT = TLI.getRegisterTypeForCallingConv(CLI.RetTy->getContext(),
+ CLI.CallConv, RetVT);
+ MVT RegVT = TLI.getRegisterType(CLI.RetTy->getContext(), RetVT);
+ if (ABIVT != RegVT)
+ return false;
+ }
}
// Call / invoke instructions with NoCfCheck attribute require special
diff --git a/llvm/test/CodeGen/X86/fast-isel-struct-ret.ll b/llvm/test/CodeGen/X86/fast-isel-struct-ret.ll
new file mode 100644
index 0000000000000..34798ef5abe1f
--- /dev/null
+++ b/llvm/test/CodeGen/X86/fast-isel-struct-ret.ll
@@ -0,0 +1,58 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -fast-isel -fast-isel-abort=3 < %s | FileCheck %s
+
+declare { i32, i32 } @get_i32s()
+
+define i32 @call_get_i32s() nounwind {
+; CHECK-LABEL: call_get_i32s:
+; CHECK: # %bb.0:
+; CHECK-NEXT: pushq %rax
+; CHECK-NEXT: callq get_i32s@PLT
+; CHECK-NEXT: addl %edx, %eax
+; CHECK-NEXT: popq %rcx
+; CHECK-NEXT: retq
+ %res = call { i32, i32 } @get_i32s()
+ %res.0 = extractvalue { i32, i32 } %res, 0
+ %res.1 = extractvalue { i32, i32 } %res, 1
+ %add = add i32 %res.0, %res.1
+ ret i32 %add
+}
+
+declare { ptr, ptr } @get_ptrs()
+
+define i64 @call_get_ptrs() nounwind {
+; CHECK-LABEL: call_get_ptrs:
+; CHECK: # %bb.0:
+; CHECK-NEXT: pushq %rax
+; CHECK-NEXT: callq get_ptrs@PLT
+; CHECK-NEXT: subq %rdx, %rax
+; CHECK-NEXT: popq %rcx
+; CHECK-NEXT: retq
+ %res = call { ptr, ptr } @get_ptrs()
+ %res.0 = extractvalue { ptr, ptr } %res, 0
+ %res.1 = extractvalue { ptr, ptr } %res, 1
+ %res.0.addr = ptrtoaddr ptr %res.0 to i64
+ %res.1.addr = ptrtoaddr ptr %res.1 to i64
+ %sub = sub i64 %res.0.addr, %res.1.addr
+ ret i64 %sub
+}
+
+declare { i64, i1 } @get_i64_and_bool()
+
+define i64 @call_get_i64_and_bool() nounwind {
+; CHECK-LABEL: call_get_i64_and_bool:
+; CHECK: # %bb.0:
+; CHECK-NEXT: pushq %rax
+; CHECK-NEXT: callq get_i64_and_bool@PLT
+; CHECK-NEXT: andb $1, %dl
+; CHECK-NEXT: movzbl %dl, %ecx
+; CHECK-NEXT: addq %rcx, %rax
+; CHECK-NEXT: popq %rcx
+; CHECK-NEXT: retq
+ %res = call { i64, i1 } @get_i64_and_bool()
+ %res.0 = extractvalue { i64, i1 } %res, 0
+ %res.1 = extractvalue { i64, i1 } %res, 1
+ %res.1.ext = zext i1 %res.1 to i64
+ %add = add i64 %res.0, %res.1.ext
+ ret i64 %add
+}
|
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
LuoYuanke
reviewed
Apr 29, 2026
| ret i64 %sub | ||
| } | ||
|
|
||
| declare { i64, i1 } @get_i64_and_bool() |
Contributor
There was a problem hiding this comment.
Could you add a test case that return { bf16, i32 }?
LuoYuanke
approved these changes
Apr 29, 2026
Contributor
LuoYuanke
left a comment
LuoYuanke
left a comment
There was a problem hiding this comment.
Thank you for fixing the compiling-time issue.
LGTM except a nit comment.
github-project-automation
Bot
moved this from Needs Triage
to Done
in LLVM Release Status
Contributor
Author
|
/cherry-pick 30fa415 |
Member
|
/pull-request #195029 |
kirthana14m
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Apr 30, 2026
After llvm#180322, X86 FastISel forces SDAG fallback for any call with a struct return. This caused major compile-time regressions for debug builds in Rust, where struct returns are very common. The type legality check should work on the de-aggregated types, not on the return type directly.
dyung
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
May 1, 2026
After llvm#180322, X86 FastISel forces SDAG fallback for any call with a struct return. This caused major compile-time regressions for debug builds in Rust, where struct returns are very common. The type legality check should work on the de-aggregated types, not on the return type directly. (cherry picked from commit 30fa415)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After #180322, X86 FastISel forces SDAG fallback for any call with a struct return. This caused major compile-time regressions for debug builds in Rust, where struct returns are very common.
The type legality check should work on the de-aggregated types, not on the return type directly.